fix(tools): separate Tape runtime capabilities#1978
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR restricts Agent Tape tools to the atomic ChangesTape Capability Layering
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Model
participant AgentToolManager
participant AgentTapeToolHandler
participant RuntimePort
Model->>AgentToolManager: callTool(tape_search / tape_context)
AgentToolManager->>AgentTapeToolHandler: isModelTool(toolName)
AgentTapeToolHandler-->>AgentToolManager: true
AgentToolManager->>AgentTapeToolHandler: call(toolName, args)
AgentTapeToolHandler->>RuntimePort: searchTape / getTapeContext
RuntimePort-->>AgentTapeToolHandler: result
AgentTapeToolHandler-->>Model: tape result
Model->>AgentToolManager: callTool(tape_handoff)
AgentToolManager->>AgentTapeToolHandler: isModelTool(tape_handoff)
AgentTapeToolHandler-->>AgentToolManager: false
AgentToolManager-->>Model: error - not available to model
sequenceDiagram
participant Renderer
participant ToolClient
participant Route
participant ToolPresenter
Renderer->>ToolClient: getConfigurableAgentToolDefinitions(context)
ToolClient->>Route: tools.listDefinitions(context)
Route->>ToolPresenter: getConfigurableAgentToolDefinitions(context)
ToolPresenter->>ToolPresenter: filter user-configurable tools
ToolPresenter-->>Route: MCPToolDefinition[]
Route-->>ToolClient: tools
ToolClient-->>Renderer: tools
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/renderer/settings/components/DeepChatAgentsSettings.vue (1)
1473-1478: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant tool source filter.
Since
getConfigurableAgentToolDefinitionsis explicitly scoped by the backend to return only user-configurable agent tools (as verified in the updated tests), filtering bytool.source === 'agent'on the client side is redundant and can be safely removed.♻️ Proposed refactor
- const definitions = await toolClient.getConfigurableAgentToolDefinitions({ chatMode: 'agent' }) - tools.value = Array.isArray(definitions) - ? definitions - .filter((tool) => tool.source === 'agent') - .sort((a, b) => a.function.name.localeCompare(b.function.name)) - : [] + const definitions = await toolClient.getConfigurableAgentToolDefinitions({ chatMode: 'agent' }) + tools.value = Array.isArray(definitions) + ? definitions.sort((a, b) => a.function.name.localeCompare(b.function.name)) + : []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/settings/components/DeepChatAgentsSettings.vue` around lines 1473 - 1478, Remove the redundant `tool.source === 'agent'` filter from the `getConfigurableAgentToolDefinitions` result handling in the settings component. Preserve the existing alphabetical sort by `a.function.name` and the empty-array fallback for non-array responses.src/shared/agentTools.ts (1)
28-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant type assertion.
Since
isTapeToolName(toolName)acts as a type guard that narrowstoolNametoTapeToolName, the explicitas TapeToolNamecast is redundant and can be safely removed.♻️ Proposed refactor
-export const getAgentToolExposure = (toolName: string): AgentToolExposure => - isTapeToolName(toolName) - ? AGENT_TOOL_EXPOSURE_BY_NAME[toolName as TapeToolName] - : 'user-configurable' +export const getAgentToolExposure = (toolName: string): AgentToolExposure => + isTapeToolName(toolName) + ? AGENT_TOOL_EXPOSURE_BY_NAME[toolName] + : 'user-configurable'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/agentTools.ts` around lines 28 - 32, Remove the redundant type assertion `as TapeToolName` from the AGENT_TOOL_EXPOSURE_BY_NAME array access within the getAgentToolExposure function. The isTapeToolName type guard already narrows the toolName parameter to TapeToolName in the true branch of the ternary, so TypeScript can infer the correct type without the explicit cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts`:
- Around line 8-19: Keep the existing fallback that catches failure to import
better-sqlite3-multiple-ciphers, but remove catch handlers from the conditional
imports of newSessions, newSessionDisabledAgentTools, and newEnvironments,
including the corresponding imports around the later migration setup. Ensure
table-module import failures propagate and fail the test suite instead of
disabling it.
---
Nitpick comments:
In `@src/renderer/settings/components/DeepChatAgentsSettings.vue`:
- Around line 1473-1478: Remove the redundant `tool.source === 'agent'` filter
from the `getConfigurableAgentToolDefinitions` result handling in the settings
component. Preserve the existing alphabetical sort by `a.function.name` and the
empty-array fallback for non-array responses.
In `@src/shared/agentTools.ts`:
- Around line 28-32: Remove the redundant type assertion `as TapeToolName` from
the AGENT_TOOL_EXPOSURE_BY_NAME array access within the getAgentToolExposure
function. The isTapeToolName type guard already narrows the toolName parameter
to TapeToolName in the true branch of the ternary, so TypeScript can infer the
correct type without the explicit cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ad60e7d-c613-444b-85ca-4ccbfc676c78
📒 Files selected for processing (47)
docs/architecture/agent-memory-system/spec.mddocs/architecture/deepchat-tape-baseline/spec.mddocs/architecture/session-boundary-cleanup/spec.mddocs/architecture/tape-capability-layering/plan.mddocs/architecture/tape-capability-layering/spec.mddocs/architecture/tape-capability-layering/tasks.mddocs/architecture/tool-system.mdscripts/architecture-guard.mjssrc/main/agent/deepchat/deepChatAgentRepository.tssrc/main/agent/shared/agentSessionNormalization.tssrc/main/agent/shared/agentSharedData.tssrc/main/presenter/agentRuntimePresenter/deferredToolExecutor.tssrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentRuntimePresenter/tapeService.tssrc/main/presenter/index.tssrc/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.tssrc/main/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook.tssrc/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/sessionApplication/projectionCoordinator.tssrc/main/presenter/startupMigrations/sessionDataMigrations.tssrc/main/presenter/toolPresenter/agentTools/agentTapeTools.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tssrc/main/presenter/toolPresenter/agentTools/index.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/toolPresenter/runtimePorts.tssrc/main/routes/index.tssrc/renderer/api/ToolClient.tssrc/renderer/settings/components/DeepChatAgentsSettings.vuesrc/renderer/src/components/chat-input/McpIndicator.vuesrc/shared/agentTools.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/tool.presenter.d.tstest/main/agent/deepchat/deepChatAgentRepository.test.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentRuntimePresenter/tapeService.test.tstest/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.tstest/main/presenter/sessionApplication/agentAssignmentPolicy.test.tstest/main/presenter/sessionApplication/projectionCoordinator.test.tstest/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.tstest/main/presenter/startupMigrations/sessionDataMigrations.test.tstest/main/presenter/toolPresenter/agentTools/agentTapeTools.test.tstest/main/presenter/toolPresenter/toolPresenter.test.tstest/main/routes/dispatcher.test.tstest/main/scripts/architectureGuard.test.tstest/renderer/api/clients.test.tstest/renderer/components/DeepChatAgentsSettings.test.tstest/renderer/components/McpIndicator.test.ts
💤 Files with no reviewable changes (2)
- src/main/presenter/index.ts
- src/main/presenter/toolPresenter/runtimePorts.ts
Comment on lines
+8
to
+19
| const sqliteModule = await import('better-sqlite3-multiple-ciphers').catch(() => null) | ||
| const sessionsModule = sqliteModule | ||
| ? await import('@/presenter/sqlitePresenter/tables/newSessions').catch(() => null) | ||
| : null | ||
| const disabledToolsModule = sqliteModule | ||
| ? await import('@/presenter/sqlitePresenter/tables/newSessionDisabledAgentTools').catch( | ||
| () => null | ||
| ) | ||
| : null | ||
| const environmentsModule = sqliteModule | ||
| ? await import('@/presenter/sqlitePresenter/tables/newEnvironments').catch(() => null) | ||
| : null |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts'
echo '== file outline =='
ast-grep outline "$file" --view expanded || true
echo
echo '== relevant lines =='
sed -n '1,140p' "$file" | cat -nRepository: ThinkInAIXYZ/deepchat
Length of output: 6987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts'
wc -l "$file"
printf '\n--- lines 1-120 ---\n'
sed -n '1,120p' "$file" | cat -n
printf '\n--- import references ---\n'
rg -n "better-sqlite3-multiple-ciphers|newSessions|newSessionDisabledAgentTools|newEnvironments|skip|describe|it|test" "$file"Repository: ThinkInAIXYZ/deepchat
Length of output: 7196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
'src/presenter/sqlitePresenter/tables/newSessions.ts' \
'src/presenter/sqlitePresenter/tables/newSessionDisabledAgentTools.ts' \
'src/presenter/sqlitePresenter/tables/newEnvironments.ts' \
'test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts'
do
if [ -f "$f" ]; then
echo "== $f =="
ast-grep outline "$f" --view expanded || true
echo
sed -n '1,220p' "$f" | cat -n | sed -n '1,220p'
echo
fi
doneRepository: ThinkInAIXYZ/deepchat
Length of output: 8304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual table module paths if they differ from the guessed src/ prefix.
fd 'newSessions\.ts|newSessionDisabledAgentTools\.ts|newEnvironments\.ts' .Repository: ThinkInAIXYZ/deepchat
Length of output: 357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
'src/main/presenter/sqlitePresenter/tables/newSessions.ts' \
'src/main/presenter/sqlitePresenter/tables/newSessionDisabledAgentTools.ts' \
'src/main/presenter/sqlitePresenter/tables/newEnvironments.ts'
do
echo "== $f =="
ast-grep outline "$f" --view expanded || true
echo
sed -n '1,220p' "$f" | cat -n | sed -n '1,220p'
echo
doneRepository: ThinkInAIXYZ/deepchat
Length of output: 20675
Do not catch the table-module imports. Lines 8-19 and 41-44: keep the better-sqlite3-multiple-ciphers fallback, but let the @/presenter/... imports fail so regressions in newSessions, newSessionDisabledAgentTools, or newEnvironments fail CI instead of skipping the suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts`
around lines 8 - 19, Keep the existing fallback that catches failure to import
better-sqlite3-multiple-ciphers, but remove catch handlers from the conditional
imports of newSessions, newSessionDisabledAgentTools, and newEnvironments,
including the corresponding imports around the later migration setup. Ensure
table-module import failures propagate and fail the test suite instead of
disabling it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Separate Tape runtime capabilities from user-configurable Agent tools.
Tape is now treated as built-in runtime infrastructure rather than an optional tool group. Model-facing recall remains available when supported, while diagnostic and runtime-only capabilities stay outside the model tool catalog and Agent settings UI.
Closes #1975
Changes
tape_search:system-modeltape_context:system-modeltape_info:diagnostictape_anchors:diagnostictape_handoff:runtime-onlytape_searchandtape_contextas an atomic recall pair.Summary by CodeRabbit
New Features
Bug Fixes
Documentation